Skip to content

Restify /contents API#600

Closed
gmelodie wants to merge 17 commits intoapi-v2from
gabe/restify-contents-api
Closed

Restify /contents API#600
gmelodie wants to merge 17 commits intoapi-v2from
gabe/restify-contents-api

Conversation

@gmelodie
Copy link
Contributor

@gmelodie gmelodie commented Nov 28, 2022

Addresses #583

Verification

  1. Add CAR, add to bucket (new name for collections) and dir too
    image
    1.2. List that bucket on that dir to see the file
    image

  2. Add CID, add to bucket and dir too
    image
    2.2. List that bucket on that dir to see the pinned file
    image

  3. Getting the status of one of the contents
    image

  4. Get content by CID
    image

@gmelodie gmelodie added the APIv2 API v2 changes label Nov 28, 2022
@gmelodie gmelodie self-assigned this Nov 28, 2022
@gmelodie gmelodie changed the base branch from dev to api-v2 November 28, 2022 14:22
@gmelodie gmelodie force-pushed the gabe/restify-contents-api branch from 4a754d1 to eebc00d Compare November 29, 2022 11:32
@gmelodie gmelodie force-pushed the gabe/restify-contents-api branch from e3b1bec to 689fdfc Compare November 30, 2022 14:02
@gmelodie gmelodie marked this pull request as ready for review November 30, 2022 19:06
@kelindi
Copy link
Contributor

kelindi commented Dec 5, 2022

@gmelodie Could we have the car files be passed through formData rather than through the body to be more consistent with how regular files are added

e.POST("/contents", withUser(s.handleAdd), s.AuthRequired(util.PermLevelUpload))

contents := e.Group("", s.AuthRequired(util.PermLevelUser))
contents.GET("/contents", withUser(s.handleListContent))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't these be behind /api/v2 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be, we will rebase after #596

Copy link
Contributor

@anjor anjor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not delete the v1 endpoints. They should still be around. The new endpoints should be under a v2 path.

Separately, I think uploadType should be an enum

@gmelodie
Copy link
Contributor Author

gmelodie commented Dec 6, 2022

@kelindi I'm trying not to change too much. I think it's about balance.

@kelindi
Copy link
Contributor

kelindi commented Dec 6, 2022

@gmelodie I think it would add some more consistency when uploading a file, which would be better for dx. If I am understanding it correctly, the current route you have is the same no matter the file and then it figures out what the case is based on the file? If that is the case it would make more sense to have the request be formatted the same when uploading a regular file or a car file.

@anjor
Copy link
Contributor

anjor commented Dec 6, 2022

No the user has to specify the upload type, it's not inferred .

@gmelodie
Copy link
Contributor Author

gmelodie commented Dec 8, 2022

Closing in favor of #716

@gmelodie gmelodie closed this Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

APIv2 API v2 changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants